pkg/services/authz/rbac/cache.go (1)

30-32: Cache key collision risk with _ separator.

The denial cache key shares the same prefix format as userPermCacheKey (namespace + ".perm_" + userUID + "_" + action), with name and parent appended using the same _ delimiter. If any field value ever contains _, keys could collide—both across the two key types and within denial keys themselves (e.g., name="a_b", parent="c" vs name="a", parent="b_c").

Current resource names/UIDs likely don't contain _, so the practical risk is low, but consider using a more robust separator (e.g., / or a multi-char delimiter) to make the key scheme collision-proof.

💡 Possible fix

 func userPermDenialCacheKey(namespace, userUID, action, name, parent string) string {
-	return namespace + ".perm_" + userUID + "_" + action + "_" + name + "_" + parent
+	return namespace + ".perm_denial_" + userUID + "/" + action + "/" + name + "/" + parent
 }

Using a distinct prefix (perm_denial_ vs perm_) and a different separator (/) eliminates any ambiguity with userPermCacheKey.

pkg/services/authz/rbac.go (1)

239-251: Receiver name lc is misleading for NoopCache.

The receiver is named lc (likely copied from a LocalCache pattern), but this is a NoopCache. Consider renaming to nc or c for consistency.

💡 Proposed fix

-func (lc *NoopCache) Get(ctx context.Context, key string) ([]byte, error) {
+func (nc *NoopCache) Get(ctx context.Context, key string) ([]byte, error) {
 	return nil, cache.ErrNotFound
 }
 
-func (lc *NoopCache) Set(ctx context.Context, key string, data []byte, exp time.Duration) error {
+func (nc *NoopCache) Set(ctx context.Context, key string, data []byte, exp time.Duration) error {
 	return nil
 }
 
-func (lc *NoopCache) Delete(ctx context.Context, key string) error {
+func (nc *NoopCache) Delete(ctx context.Context, key string) error {
 	return nil
 }

pkg/services/authz/rbac/service.go (2)

342-368: getCachedIdentityPermissions performs a DB call for user identifiers, partially negating the "cache-only" intent.

For TypeUser/TypeServiceAccount, GetUserIdentifiers (line 356) may hit the database if the identity isn't in idCache. This means the "fast cached path" can still trigger a DB round-trip just to construct the perm-cache key, even if the perm cache itself turns out to be empty. The idCache has a longer TTL (2 min) so this is mostly mitigated after the first request, but on a cold cache or after TTL expiry, every Check/List call pays this cost upfront.

If this is intentional, a brief code comment explaining the tradeoff would help future maintainers.

---

153-155: Denial cache entries may outlive the permission cache, creating a brief stale-denial window.

Both permCache and permDenialCache use shortCacheTTL (30s), but the denial entry is written after the perm entry (which is set inside getUserPermissions). When a permission is granted in the DB shortly after a denial is cached, the denial entry can outlive the perm entry by the delta between the two writes, causing the fast-path at line 117 to return a stale denial even after permCache refreshes.

In practice this delta is small (milliseconds), and the 30s TTL makes any staleness transient. However, it's worth documenting this known tradeoff (or setting the denial TTL slightly shorter than the perm TTL) so operators understand the behavior during permission changes.

pkg/services/authz/rbac/service_test.go (1)

973-995: Test doesn't conclusively prove the denial cache short-circuits.

The permCache entry has "dashboards:uid:dash1": false, which would also result in a denial through the normal permission-check path. If the denial cache were removed, the test would still pass (via the perm check + DB fallback path). To truly verify that the denial cache takes priority, set a permCache value that would allow the request:

-		s.permCache.Set(ctx, userPermCacheKey("org-12", "test-uid", "dashboards:read"), map[string]bool{"dashboards:uid:dash1": false})
+		s.permCache.Set(ctx, userPermCacheKey("org-12", "test-uid", "dashboards:read"), map[string]bool{"dashboards:uid:dash1": true})

This way, if the denial cache is bypassed, checkPermission would return allowed=true, and the test would fail—proving the denial cache is the decisive factor.